Skip to content

Add input validation to Subject.set_color_depth - #382

Closed
snowplow-claude-review[bot] wants to merge 1 commit into
masterfrom
loop/3a307af295a281a9ba70f6d2fa691c78-snowplow-python-tracker-29728427289
Closed

Add input validation to Subject.set_color_depth#382
snowplow-claude-review[bot] wants to merge 1 commit into
masterfrom
loop/3a307af295a281a9ba70f6d2fa691c78-snowplow-python-tracker-29728427289

Conversation

@snowplow-claude-review

Copy link
Copy Markdown

🤖 Draft implementation — refinement loop (3a307af295a281a9ba70f6d2fa691c78)

Drafted automatically by the Snowplow refinement loop (refine → product review → implement → review) from a Notion refinement, and opened here for a human to finish.

Heads-up: this is a starting point. A human reviewer should verify, finish, and run CI before merging.

What the loop did

Add input validation to Subject.set_color_depth

Subject.set_color_depth(depth) accepted any integer, including zero and negatives, which are nonsensical screen colour-depth values. The sibling setters set_screen_resolution and set_viewport already guard their inputs with the repo's greater_than contract helper; set_color_depth was missing the same guard.

Changes

snowplow_tracker/subject.py

  • Added greater_than(depth, 0) as the first statement of set_color_depth, raising ValueError for zero and negative inputs. Reuses the existing import on line 19 — no new error-handling patterns introduced.
  • Updated the :type depth: docstring annotation from int to int,>0 to match sibling setters.

snowplow_tracker/test/unit/test_subject.py

  • Added test_set_color_depth_validation to TestSubject, asserting ValueError for depth=0 and depth=-1, and no exception for depth=24.

Key decisions

  • Reused the existing greater_than contract helper — consistent with set_screen_resolution and set_viewport, no new patterns.
  • Scope held to set_color_depth only; no other setters touched.
  • No wire-format or schema change — the cd field is unchanged; only invalid values are now rejected at the SDK boundary.

Review checklist

  • greater_than(depth, 0) is the first statement in set_color_depth, before any assignment.
  • The docstring annotation reads int,>0.
  • test_set_color_depth_validation covers 0 and -1 (expect ValueError) and 24 (expect no exception).
  • All existing tests pass (142 unit tests confirmed green).
  • No changes outside the two files listed above.

…2fa691c78-snowplow-python-tracker-29728427289)
@snowplowcla

Copy link
Copy Markdown

Thanks for your pull request. Is this your first contribution to a Snowplow open source project? Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://docs.snowplowanalytics.com/docs/contributing/contributor-license-agreement/ to learn more and sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

@snowplowcla Snowplow CLA bot (snowplowcla) added the cla:no [Auto generated] Snowplow Contributor License Agreement has not been signed. label Jul 20, 2026
@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 96.629% (+0.01%) from 96.619% — loop/3a307af295a281a9ba70f6d2fa691c78-snowplow-python-tracker-29728427289 into master

@matus-tomlein
Matus Tomlein (matus-tomlein) deleted the loop/3a307af295a281a9ba70f6d2fa691c78-snowplow-python-tracker-29728427289 branch July 20, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla:no [Auto generated] Snowplow Contributor License Agreement has not been signed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants